Enable IO and UpdateTest types #15104
Conversation
| @@ -1,22 +1,24 @@ | |||
| # typed: false | |||
| # typed: true | |||
| # frozen_string_literal: true | |||
|
|
|||
| class IO | |||
| def readline_nonblock(sep = $INPUT_RECORD_SEPARATOR) | |||
| line = +"" | |||
| buffer = +"" | |||
There was a problem hiding this comment.
this needs to be outside the rescued block, so that line is provably a String when line.empty? is invoked below.
| raise e if line.empty? | ||
| line.freeze | ||
| rescue IO::WaitReadable, EOFError | ||
| raise if line.empty? |
There was a problem hiding this comment.
IO::WaitReadable is not technically an Exception, so it can't be passed to raise, but we can avoid the issue using an empty re-raise instead (e is not needed otherwise)
dduugg
force-pushed
the
enable-io-types
branch
3 times, most recently
from
bcfcb64
to
fa0caaa
Compare
dduugg
changed the title
Very interesting! |
| end_commit = "HEAD" | ||
| cd HOMEBREW_REPOSITORY do | ||
| start_commit = if (commit = args.commit) | ||
| start_commit = if (commit = T.let(args.commit, T.nilable(String))) |
There was a problem hiding this comment.
Is this really necessary? I tested it locally and it didn't seem to care.
There was a problem hiding this comment.
Yeh, are some of these changes only needed for typed: strict @dduugg?
There was a problem hiding this comment.
Ah, sorry, this is a vestige of attempting to type the Utils.popen* methods. Would you like me to remove it in the next typing PR?
| end_commit = "HEAD" | ||
| cd HOMEBREW_REPOSITORY do | ||
| start_commit = if (commit = args.commit) | ||
| start_commit = if (commit = T.let(args.commit, T.nilable(String))) |
There was a problem hiding this comment.
Yeh, are some of these changes only needed for typed: strict @dduugg?
|
Thanks again @dduugg! Happy to chat more post-merge. |
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?Enables types in Library/Homebrew/extend/io.rb and Library/Homebrew/dev-cmd/update-test.rb
Note that this is a bit clumsy in one case, where we mark an lvar as
untyped. This is becauseUtils.popen_readresists a straightforwardsig(The return type is the return type of the block given to it, or aStringotherwise. Sorbet allows multiple sigs in the Ruby core and std libraries in order to express this, but doesn't allow it otherwise.)